-
Notifications
You must be signed in to change notification settings - Fork 12.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support s390x z13 vector ABI #131586
base: master
Are you sure you want to change the base?
Support s390x z13 vector ABI #131586
Conversation
This comment has been minimized.
This comment has been minimized.
r? compiler |
7656805
to
07ca922
Compare
This comment has been minimized.
This comment has been minimized.
c14d9b2
to
d39e822
Compare
Fixed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition to the problem mentioned inline, we should also verify that building with soft-float works as expected. This option implicitly disables any use of floating-point or vector registers, and therefore also disables use of the vector facility. I believe the LLVM back-end should handle that logic correctly, but it would be good to have the test check.
tests/assembly/s390x-vector-abi.rs
Outdated
*x | ||
} | ||
|
||
// FIXME: should check output for z10, but it is very long... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's also incorrect. Without the vector feature, vector types of any length are supposed to be passed via implicit reference, which in the case means a pointer in %r2. The body of the function should simply be:
lg %r2, 0(%r2)
Not sure what exactly the compiler is doing here, but it appears to have scalarized the input and pass it as 8 distinct one-byte arguments ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This case and soft-float case should eventually result in a compilation error once the ABI check is implemented, and I will add UI tests for these cases once the ABI check is implemented.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough. I just wanted to point out the incorrect code, in case we do end up going back to supporting the no-vector case.
0a6b7ad
to
c1835bd
Compare
This comment has been minimized.
This comment has been minimized.
c1835bd
to
f4f0361
Compare
This comment has been minimized.
This comment has been minimized.
f4f0361
to
e18682e
Compare
This comment has been minimized.
This comment has been minimized.
e18682e
to
ec20e94
Compare
This comment has been minimized.
This comment has been minimized.
ec20e94
to
8c4ec03
Compare
This comment has been minimized.
This comment has been minimized.
546fd91
to
857dbc6
Compare
This comment has been minimized.
This comment has been minimized.
857dbc6
to
42a4984
Compare
This comment has been minimized.
This comment has been minimized.
42a4984
to
1a8d1aa
Compare
This comment has been minimized.
This comment has been minimized.
a9242d0
to
524c9f6
Compare
☔ The latest upstream changes (presumably #131747) made this pull request unmergeable. Please resolve the merge conflicts. |
On some architectures, vector types may have a different ABI when relevant target features are enabled. As discussed in rust-lang/lang-team#235, this turns out to very easily lead to unsound code. This commit makes it an error to declare or call functions using those vector types in a context in which the corresponding target features are disabled, if using an ABI for which the difference is relevant.
524c9f6
to
c28dc57
Compare
cc #130869
This resolves the following fixmes:
rust/compiler/rustc_target/src/abi/call/s390x.rs
Lines 1 to 2 in 58420a0
rust/compiler/rustc_target/src/spec/targets/s390x_unknown_linux_gnu.rs
Lines 9 to 11 in 58420a0
Refs: Section 1.2.3 "Parameter Passing" and section 1.2.5 "Return Values" in ELF Application Binary Interface s390x Supplement, Version 1.6.1 (lzsabi_s390x.pdf in https://github.com/IBM/s390x-abi/releases/tag/v1.6.1)
Depends on #127731. (The first commit is from it and the second commit is fix errors happened after rebased it.)
This PR extends #127731 's ABI check for that PR to reject cases where
vector
target feature is disabled.If we do not do ABI check, we run into the ABI problems as described in #116558 and #130869 (comment), and the problem of the compiler generating strange code (#131586 (comment)).
cc @uweigand
@rustbot label +O-SystemZ +A-ABI